datacube: enable saving and loading local file source in DataCube grid #3903

gs-gunjan · 2025-02-20T15:30:04Z

Summary

How did you test this change?

Test(s) added
Manual testing (please provide screenshots/recordings)
No testing (please provide an explanation)

changeset-bot · 2025-02-20T15:30:08Z

🦋 Changeset detected

Latest commit: e2aedd6

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 28 packages

Name	Type
@finos/legend-application-data-cube	Patch
@finos/legend-data-cube	Patch
@finos/legend-application-data-cube-bootstrap	Patch
@finos/legend-application-query	Patch
@finos/legend-application-repl	Patch
@finos/legend-application-studio	Patch
@finos/legend-query-builder	Patch
@finos/legend-vscode-extension-dependencies	Patch
@finos/legend-application-data-cube-deployment	Patch
@finos/legend-application-query-bootstrap	Patch
@finos/legend-extension-dsl-service	Patch
@finos/legend-application-repl-deployment	Patch
@finos/legend-application-studio-bootstrap	Patch
@finos/legend-extension-assortment	Patch
@finos/legend-extension-dsl-data-quality	Patch
@finos/legend-extension-dsl-data-space-studio	Patch
@finos/legend-extension-dsl-diagram	Patch
@finos/legend-extension-dsl-persistence	Patch
@finos/legend-extension-dsl-text	Patch
@finos/legend-extension-store-flat-data	Patch
@finos/legend-extension-store-relational	Patch
@finos/legend-extension-store-service-store	Patch
@finos/legend-extension-dsl-data-space	Patch
@finos/legend-application-query-deployment	Patch
@finos/legend-application-studio-deployment	Patch
@finos/legend-application-pure-ide	Patch
@finos/legend-application-pure-ide-deployment	Patch
@finos/legend-server-showcase-deployment	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

codecov · 2025-02-20T15:34:09Z

Codecov Report

Attention: Patch coverage is 0% with 671 lines in your changes missing coverage. Please review.

Project coverage is 46.17%. Comparing base (2fa1cbb) to head (e2aedd6).
Report is 4 commits behind head on master.

Files with missing lines	Patch %	Lines
...oader/LocalFileDataCubeSourceLoaderBuilderState.ts	0.00%	210 Missing and 1 partial ⚠️
...stores/builder/LegendDataCubeSourceLoaderState.tsx	0.00%	120 Missing ⚠️
...ta-cube/src/stores/LegendDataCubeDataCubeEngine.ts	0.00%	89 Missing ⚠️
.../components/builder/LegendDataCubeSourceLoader.tsx	0.00%	63 Missing ⚠️
...er/source/loader/LocalFileDataCubeSourceLoader.tsx	0.00%	58 Missing ⚠️
...data-cube/src/stores/LegendDataCubeDuckDBEngine.ts	0.00%	46 Missing ⚠️
...e/loader/LegendDataCubeSourceLoaderBuilderState.ts	0.00%	37 Missing and 1 partial ⚠️
.../src/stores/builder/LegendDataCubeBuilderStore.tsx	0.00%	25 Missing ⚠️
...lder/source/LocalFileDataCubeSourceBuilderState.ts	0.00%	10 Missing ⚠️
...a-cube/src/stores/model/LocalFileDataCubeSource.ts	0.00%	8 Missing ⚠️
... and 3 more

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3903      +/-   ##
==========================================
- Coverage   46.20%   46.17%   -0.03%     
==========================================
  Files        2262     2269       +7     
  Lines      396395   397672    +1277     
  Branches    11908    16401    +4493     
==========================================
+ Hits       183158   183639     +481     
- Misses     212566   213270     +704     
- Partials      671      763      +92

Files with missing lines	Coverage Δ
...e/src/components/builder/LegendDataCubeBuilder.tsx	`0.00% <0.00%> (ø)`
.../builder/source/LocalFileDataCubeSourceBuilder.tsx	`0.00% <0.00%> (ø)`
...cube/src/stores/core/model/CachedDataCubeSource.ts	`74.07% <0.00%> (-0.93%)`	⬇️
...a-cube/src/stores/model/LocalFileDataCubeSource.ts	`0.00% <0.00%> (ø)`
...lder/source/LocalFileDataCubeSourceBuilderState.ts	`0.00% <0.00%> (ø)`
.../src/stores/builder/LegendDataCubeBuilderStore.tsx	`0.00% <0.00%> (ø)`
...e/loader/LegendDataCubeSourceLoaderBuilderState.ts	`0.00% <0.00%> (ø)`
...data-cube/src/stores/LegendDataCubeDuckDBEngine.ts	`0.00% <0.00%> (ø)`
...er/source/loader/LocalFileDataCubeSourceLoader.tsx	`0.00% <0.00%> (ø)`
.../components/builder/LegendDataCubeSourceLoader.tsx	`0.00% <0.00%> (ø)`
... and 3 more

... and 411 files with indirect coverage changes

gs-gunjan · 2025-02-21T08:53:01Z

akphi · 2025-02-21T16:37:03Z

packages/legend-data-cube/src/stores/core/model/CachedDataCubeSource.ts

 import { DataCubeSource } from './DataCubeSource.js';
 import type { PlainObject } from '@finos/legend-shared';

 export class CachedDataCubeSource extends DataCubeSource {
-  model!: PlainObject<V1_PureModelContextData>;
+  model!: PlainObject;


why this change? It's not useful type-checking wise, but it's a coding guide

akphi · 2025-02-21T16:37:27Z

packages/legend-application-data-cube/src/stores/model/LocalFileDataCubeSource.ts

@@ -31,7 +30,7 @@ export enum LocalFileDataCubeSourceFormat {
 }

 export class LocalFileDataCubeSource extends DataCubeSource {
-  model!: PlainObject<V1_PureModelContextData>;
+  model!: PlainObject;


ditto, revert this

akphi · 2025-02-21T16:39:49Z

...egend-application-data-cube/src/components/builder/source/LocalFileDataCubeSourceBuilder.tsx

@@ -31,7 +31,7 @@ export const LocalFileDataCubeSourceBuilder = observer(
          text={`Currently, support for local file comes with the following limitations:
 - Only CSV files are supported, but not all variants of CSV files are supported (required header row, comma delimiter, single escape quote).
 - Data from uploaded file will not be stored nor shared.
- DataCube created with local file source cannot be saved.`}
+- DataCube from uploaded file can be stored but when loading this, you will have to reupload source data.`}


I think we can remove this line.

akphi · 2025-02-21T16:40:41Z

...ication-data-cube/src/stores/builder/source/loader/LegendDataCubeSourceLoaderBuilderState.ts

+import type { LegendDataCubeApplicationStore } from '../../../LegendDataCubeBaseStore.js';
+import type { LegendDataCubeDataCubeEngine } from '../../../LegendDataCubeDataCubeEngine.js';
+
+export abstract class LegendDataCubeSourceLoaderBuilderState {


Rename this and the file to LegendDataCubeSourceLoaderState

akphi · 2025-02-21T16:41:28Z

...tion-data-cube/src/stores/builder/source/loader/LocalFileDataCubeSourceLoaderBuilderState.ts

+} from '../../../model/LocalFileDataCubeSource.js';
+import { LegendDataCubeSourceLoaderBuilderState } from './LegendDataCubeSourceLoaderBuilderState.js';
+
+export class LocalFileDataCubeSourceLoaderBuilderState extends LegendDataCubeSourceLoaderBuilderState {


LocalFileDataCubeSourceLoaderBuilderState -> LocalFileDataCubeSourceLoaderState

akphi · 2025-02-21T18:40:10Z

packages/legend-application-data-cube/src/stores/builder/LegendDataCubeSourceLoaderState.tsx

+    return false;
+  }
+
+  changeSourceBuilder(type: string): void {


Mindful and scan everywhere you copy code, change builder to loader

Suggested change

changeSourceBuilder(type: string): void {

changeSourceLoader(type: string): void {

akphi · 2025-02-21T18:41:38Z

...ication-data-cube/src/stores/builder/source/loader/LegendDataCubeSourceLoaderBuilderState.ts

+
+  abstract generateSourceData(): Promise<PlainObject>;
+
+  abstract validateSourceData(source: PlainObject | undefined): boolean;


as commented in LocalFile...SourceLoader, the spirit is wrong here, we don't need this method

akphi · 2025-02-21T18:42:44Z

...ication-data-cube/src/stores/builder/source/loader/LegendDataCubeSourceLoaderBuilderState.ts

+
+  abstract get isValid(): boolean;
+
+  abstract generateSourceData(): Promise<PlainObject>;


Copying the code from sourcebuilder is not 100% appropriate here, we are NOT trying to generate new source, we're trying to loadSource, this method should be changed to

Suggested change

abstract generateSourceData(): Promise<PlainObject>;

abstract load(sourceData: PlainObject): Promise<PlainObject>;

akphi · 2025-02-21T18:46:41Z

packages/legend-application-data-cube/src/stores/builder/LegendDataCubeBuilderStore.tsx

@@ -295,6 +299,27 @@ export class LegendDataCubeBuilderStore {
        const specification = DataCubeSpecification.serialization.fromJson(
          persistentDataCube.content,
        );
+
+        if (
+          !this.saveState.hasSucceeded &&


why we use saveState here??

akphi · 2025-02-21T18:57:07Z

packages/legend-application-data-cube/src/stores/builder/LegendDataCubeBuilderStore.tsx

+          this.sourceLoader.changeSourceBuilder(
+            specification.source._type as string,
+          );
+          this.sourceLoader.setSource(specification.source);


In terms of UX and componentization, I think this is not accurate. What we want is not yet another window on top of the current window for people to pick the source, this is more the second stage of the wizard to load query. We have to follow the following flow:

(1) When people load /datacube/:id: if we spot a partial source, we could redirect to /datacube and open the loader and fast-forward to the wizard step in the loader where people have to provide the source, we need to do some special trick to not get stuck in a loop (e.g. after loading the partial source, user redirected to /datacube/:id and hence infinite loop ) <-- need to think what we do here? Or we can leave people on the same page /datacube/:id and do something.

(2) And the other flow, in loader, when people just casually working in DataCube and decide to click Load DataCube, the loader can come up and people can select a DataCube with partial source, that's another flow you have to handle

It's best to think of a strategy to homogenize these 2 flows and just have (1) uses the logic in (2)

gs-gunjan added 2 commits February 19, 2025 15:55

datacube: enable saving local file source datacube grid (phase 2)

9edbc26

datacube: enable saving local file source datacube grid (phase 2)

0827893

datacube: enable saving local file source datacube grid (phase 2)

4828607

gs-gunjan force-pushed the csv branch from f62aa21 to 4828607 Compare February 21, 2025 08:43

gs-gunjan marked this pull request as ready for review February 21, 2025 08:46

gs-gunjan requested a review from a team as a code owner February 21, 2025 08:46

datacube: cleanup old ingest data when we load a new query

e2aedd6

akphi requested changes Feb 21, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

datacube: enable saving and loading local file source in DataCube grid #3903

datacube: enable saving and loading local file source in DataCube grid #3903

gs-gunjan commented Feb 20, 2025

changeset-bot bot commented Feb 20, 2025 •

edited

Loading

codecov bot commented Feb 20, 2025 •

edited

Loading

gs-gunjan commented Feb 21, 2025

akphi Feb 21, 2025

akphi Feb 21, 2025

akphi Feb 21, 2025

akphi Feb 21, 2025

akphi Feb 21, 2025

akphi Feb 21, 2025

akphi Feb 21, 2025

akphi Feb 21, 2025

akphi Feb 21, 2025

akphi Feb 21, 2025

	changeSourceBuilder(type: string): void {
	changeSourceLoader(type: string): void {


		abstract generateSourceData(): Promise<PlainObject>;

		abstract validateSourceData(source: PlainObject \| undefined): boolean;


		abstract get isValid(): boolean;

		abstract generateSourceData(): Promise<PlainObject>;

	abstract generateSourceData(): Promise<PlainObject>;
	abstract load(sourceData: PlainObject): Promise<PlainObject>;

datacube: enable saving and loading local file source in DataCube grid #3903

Are you sure you want to change the base?

datacube: enable saving and loading local file source in DataCube grid #3903

Conversation

gs-gunjan commented Feb 20, 2025

Summary

How did you test this change?

changeset-bot bot commented Feb 20, 2025 • edited Loading

🦋 Changeset detected

codecov bot commented Feb 20, 2025 • edited Loading

Codecov Report

gs-gunjan commented Feb 21, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

changeset-bot bot commented Feb 20, 2025 •

edited

Loading

codecov bot commented Feb 20, 2025 •

edited

Loading